Do we need List (list) or could we go with Sequence as bit more generic.

I have changed to Sequence, but I am wondering about the discussion about the Sequence type not including np.ndarray from here: Qiskit/qiskit#8812 (comment)

Even though it says Sequence the primitives will keep supporting np.ndarray. Some people suggest moving back to Iterable, however these can actually be infinite (e.g. itertools.count), which raises concerns of secutiry, memory usage, and potential DOS attacks among others.

An alternative that I have proposed in the past is to move to Collection which does include np.ndarray but without being infinite. I personally like this approach, even though Mappings are included so a dictionary could be passed.

For now, we are sticking to Sequence in the primitives since it is the least general. That way, generalizing it later on in the future will not be a breaking a change.

I think the descriptions here could include what happens for None (default) case.

Public instance vars are what we have been moving towards - they do need an Attributes: docstring part in the class description though to make them visible in the API ref,

I wonder if I should migrate all the existing properties to public instances then, I guess this would also affect the NeuralNetwork base class, because some properties like input_gradients are inherited from it.

See constructor for more details.

I think there is less stated there than here from what I can see. Rather than "only used" would it be better to revert it say "its ignored if no custom interpret method is provided where the shape is taken to be 2^circuit.num_qubits". If we state its ignored I guess the logger.warning is still valid - ideally this would not be set alone.

Updated in the constructor and here in the property as you suggested.

I'll comment here even though I think where it could be altered is the EstimatorQNN line. If I look at the summary
then EstimatorQNN is the only one in the list with neural network not with caps. Its pretty minor in the scheme of things just something that stood out with them next to one another like that,

Updated to make lower case in the QNNs. I don't know why caps.

This comes out with a space like the source above - was that intended?
As a highlight term I would have thought not.

Removed the space.

In EstimatorQNN we made TorchConnector a class ref so it was a link not just highlighted

Updated the link to TorchConnector

Even though there are just getters here (circuit, input & weight params) what are being returned are the internal references to these mutable objects. Hence while you cannot change the whole item the item itself can be mutated by what is given back. While the internal reference could be copied maybe its not needed - its not done across the constructor either unless the list() cast needs to do more.

I'd avoid copying objects until it is required. There are many other ways how to shoot in the foot.

I will note that many of the types in the arg list are not clickable links here - just the purple colored ones. I am not sure whether its an aspect of the new syntax or not. If I look at CircuitQNN QuantumCircuit is link there whereas not here. I'll check with something newer from Nature, though one from 10 days ago has a similar issue in terms of some things are links others not.

I don't know why as well. Perhaps due to the new syntax.

Is there a reason not to have SamplerQNN and CircuitQNN instead of having spaces in there. The former will match a search the other not so much. I see it was done as Oflow QNN with a space in the Estimator release note though EstimatorQNN I see just as the one word.

I don't see a reason. Updated with the links.
I can separately update the EstimatorQNN reno if required. Let me know.